-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Silence several Hint[Performance] warnings #23003
Conversation
implicit copy; if possible, rearrange your program's control flow to prevent it [Performance]` warnings that trigger without the `move`s.
Add one for `parseHeader` 's `parseList` instead.
lib/pure/httpcore.nim
Outdated
@@ -234,7 +234,7 @@ func parseList(line: string, list: var seq[string], start: int): int = | |||
while start+i < line.len and line[start + i] notin {'\c', '\l'}: | |||
i += line.skipWhitespace(start + i) | |||
i += line.parseUntil(current, {'\c', '\l', ','}, start + i) | |||
list.add(current) | |||
list.add(move current) | |||
if start+i < line.len and line[start + i] == ',': | |||
i.inc # Skip , | |||
current.setLen(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is now unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure that isn't pending "near-term/not yet merged" work? I still get this with HEAD of nim-devel compiled 15 minutes ago:
shell$ nim r --hint[Performance]=on --mm:orc tests/stdlib/thttpcore.nim
/usr/lib/nim/lib/pure/httpcore.nim(167, 25) Hint: passing 'headers.table[toCaseInsensitive(headers, key)]' to a sink parameter introduces an implicit copy; if possible, rearrange your program's control flow to prevent it [Performance]
/usr/lib/nim/lib/pure/httpcore.nim(237, 14) Hint: passing 'current' to a sink parameter introduces an implicit copy; if possible, rearrange your program's control flow to prevent it [Performance]
Hint: mm: orc; opt: none (DEBUG BUILD, `-d:release` generates faster code)
47489 lines; 0.641s; 71.609MiB peakmem; proj: thttpcore; out: ....
The line 237 one is the relevant warning, of course. (Not sure how to fix the line 167 one.) Anyway, happy to remove the line, if it really won't be necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well move current
does set current
to "" already and so the setLen 0
should not be required.
I don't see how the current CI failures relate to this PR. Maybe those failures are endemic lately? |
Thanks for your hard work on this PR! Hint: mm: orc; opt: speed; options: -d:release |
With
--mm:arc
one gets the "implicit copy; if possible, rearrange your program's control flow"Performance
warnings without thesemove
s.